Skip to content

feat(openai): support handoffs#82

Merged
shijiesheng merged 3 commits intocadence-workflow:mainfrom
shijiesheng:openai-handoff
Apr 2, 2026
Merged

feat(openai): support handoffs#82
shijiesheng merged 3 commits intocadence-workflow:mainfrom
shijiesheng:openai-handoff

Conversation

@shijiesheng
Copy link
Copy Markdown
Member

What changed?

  • add CadenceHandoff
  • replace Handoff with CadenceHandoff before invoke model and change it back after invocation.

Why?

handoff is not serializable. We need an Cadence friendly entity in between

How did you test it?

Locally tested with a handoff agent. I'll upload the demo to cadence samples later when we have the new release
Screenshot 2026-04-01 at 11 01 29 AM

Potential risks

Release notes

Documentation Changes



def from_cadence_handoff(ch: CadenceHandoff) -> Handoff[Any, Any]:
async def noop_invoke(_ctx: RunContextWrapper[Any], _json: str) -> Awaitable[Any]: # type: ignore[return]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Bug: Incorrect return type on async noop_invoke callback

The noop_invoke function is declared as async def with return type -> Awaitable[Any]. Since async def functions implicitly wrap their return value in a coroutine (which is an Awaitable), this annotation means the actual type at the call site would be Awaitable[Awaitable[Any]], which is incorrect. The # type: ignore[return] comment is masking this issue. The return type should be -> Any (or -> None since it only raises).

Suggested fix:

async def noop_invoke(_ctx: RunContextWrapper[Any], _json: str) -> Any:
    raise RuntimeError("Handoff invocation is handled by the runner, not the model")

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

Comment on lines +5 to +6
cadence_registry = cadence.Registry()
cadence_registry.register_activities(OpenAIActivities())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Module-level side effect in cadence_registry.py

cadence_registry.py instantiates OpenAIActivities() and registers it at import time. This means merely importing the module triggers side effects (object creation, registration). Consider using a factory function or lazy initialization pattern so consumers have explicit control over when registration happens.

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected

Signed-off-by: Shijie Sheng <liouvetren@gmail.com>
Signed-off-by: Shijie Sheng <liouvetren@gmail.com>
Signed-off-by: Shijie Sheng <liouvetren@gmail.com>
@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 1, 2026

Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Adds OpenAI handoff support to the Cadence Python client. Consider removing the unused Awaitable import, fixing the noop_invoke return type annotation (async def should return Coroutine not Awaitable), and deferring the OpenAIActivities instantiation to avoid module-level side effects at import time.

💡 Bug: Incorrect return type on async noop_invoke callback

📄 cadence/contrib/openai/cadence_handoff.py:33 📄 cadence/contrib/openai/cadence_handoff.py:6

The noop_invoke function is declared as async def with return type -> Awaitable[Any]. Since async def functions implicitly wrap their return value in a coroutine (which is an Awaitable), this annotation means the actual type at the call site would be Awaitable[Awaitable[Any]], which is incorrect. The # type: ignore[return] comment is masking this issue. The return type should be -> Any (or -> None since it only raises).

Suggested fix
async def noop_invoke(_ctx: RunContextWrapper[Any], _json: str) -> Any:
    raise RuntimeError("Handoff invocation is handled by the runner, not the model")
💡 Quality: Module-level side effect in cadence_registry.py

📄 cadence/contrib/openai/cadence_registry.py:5-6

cadence_registry.py instantiates OpenAIActivities() and registers it at import time. This means merely importing the module triggers side effects (object creation, registration). Consider using a factory function or lazy initialization pattern so consumers have explicit control over when registration happens.

🤖 Prompt for agents
Code Review: Adds OpenAI handoff support to the Cadence Python client. Consider removing the unused `Awaitable` import, fixing the `noop_invoke` return type annotation (async def should return `Coroutine` not `Awaitable`), and deferring the `OpenAIActivities` instantiation to avoid module-level side effects at import time.

1. 💡 Bug: Incorrect return type on async noop_invoke callback
   Files: cadence/contrib/openai/cadence_handoff.py:33, cadence/contrib/openai/cadence_handoff.py:6

   The `noop_invoke` function is declared as `async def` with return type `-> Awaitable[Any]`. Since `async def` functions implicitly wrap their return value in a coroutine (which is an `Awaitable`), this annotation means the actual type at the call site would be `Awaitable[Awaitable[Any]]`, which is incorrect. The `# type: ignore[return]` comment is masking this issue. The return type should be `-> Any` (or `-> None` since it only raises).

   Suggested fix:
   async def noop_invoke(_ctx: RunContextWrapper[Any], _json: str) -> Any:
       raise RuntimeError("Handoff invocation is handled by the runner, not the model")

2. 💡 Quality: Module-level side effect in cadence_registry.py
   Files: cadence/contrib/openai/cadence_registry.py:5-6

   cadence_registry.py instantiates `OpenAIActivities()` and registers it at import time. This means merely importing the module triggers side effects (object creation, registration). Consider using a factory function or lazy initialization pattern so consumers have explicit control over when registration happens.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Copy link
Copy Markdown
Member

@timl3136 timl3136 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shijiesheng shijiesheng merged commit 1666057 into cadence-workflow:main Apr 2, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants